Pull request overview

This PR adds new Durable Task client capabilities for bulk instance enumeration and orchestration history retrieval, along with a new public com.microsoft.durabletask.history domain model and conversion layer from protobuf history events.

Changes:

• Adds DurableTaskClient.listInstanceIds(ListInstanceIdsQuery) and supporting query/result types for paging terminal instance IDs by completion-time window and status.
• Adds DurableTaskClient.getOrchestrationHistory(String) backed by the sidecar StreamInstanceHistory gRPC operation, returning a typed public history-event model.
• Introduces HistoryEventConverter plus unit/integration tests for history conversion and the new client APIs.

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Left a few comments. Thanks for the PR!

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Summary

Strong, well-structured PR. The domain model is clean and immutable, and the protobuf→model converter is exactly correct — I verified all 28 HistoryEvent oneof cases (fields 3–30) and sampled event fields against orchestrator_service.proto, and every mapping and null-guard lines up. Test coverage is good. A handful of things worth resolving before merge, none catastrophic.

Strengths

• Converter correctness: all 28 event types handled; stringOrNull(hasX, getX) correctly guards the StringValue-wrapped optional fields. One unit test per event type (608 lines), including null-handling and the EVENTTYPE_NOT_SET → IllegalArgumentException branch.
• Model quality: event classes are final, fully immutable, @Nullable-annotated, well-documented; tags is defensively copied (Collections.unmodifiableMap(new HashMap<>(tags)), null→emptyMap).
• Back-compat: base-class defaults throw UnsupportedOperationException, so external DurableTaskClient subclasses still compile; the sole impl (DurableTaskGrpcClient) overrides both.
• ListInstanceIdsQuery validates pageSize ≥ 1 and defensively copies the status list; ListInstanceIdsResult returns an unmodifiable list.

Findings (by severity)

1. getOrchestrationHistory — forward-compat + stream cleanup (Medium). The converter throws on any unrecognized event type. A future server adding a new event type makes the entire history read fail with UnsupportedOperationException. Worse, the blocking gRPC server-streaming iterator is drained in a while(hasNext) loop with no try/finally; if fromProto throws mid-stream the loop exits by exception and the streaming RPC is abandoned without cancellation — a known grpc-java leak pattern. Recommend (a) deciding forward-compat behavior (skip / GenericEvent fallback vs. fail-fast), and (b) cancelling the call on early exit.

2. Whole history materialized in memory (Medium / design). The API returns List<HistoryEvent>, buffering the full history even though the server streams it in chunks — this defeats streaming for very large histories. For the archival/export use case, consider an Iterable/streaming/callback shape, or explicitly document the in-memory materialization.

3. Pagination termination — null vs. empty token (Low–Medium). continuationToken becomes null only when the server omits lastInstanceKey. If the server instead returns a present-but-empty StringValue on the last page, the client hands back "" (non-null); a caller looping on != null re-requests with "", which restarts from the beginning → potential infinite paging loop. Recommend treating empty as end-of-results: hasLastInstanceKey() && !value.isEmpty() ? value : null. No multi-page pagination test exists to catch this.

4. Public constructors on ~30 model types (Low / API design). Because HistoryEventConverter sits in com.microsoft.durabletask while the model lives in ...history, every event constructor must be public — pushing long positional constructors (e.g. ExecutionStartedEvent has 11 params) into the permanent public API even though users only ever read these. Moving the converter into the history package (or a package-private factory there) would let the constructors be package-private, keeping the public surface read-only.

5. Naming (Low). listInstanceIds doesn't convey "terminal instances filtered by completion time." The Javadoc clarifies, but the method name is the real contract and it's still unreleased — consider something like listCompletedInstanceIds.

6. CHANGELOG not updated (Low). New public API but no Unreleased entry; the PR checklist item is unchecked.

7. Unrelated scope (Nit). <h3>→<h2> Javadoc edits in EntityQueryPageable, TypedEntityMetadata, TypedEntityQueryPageable (doclint heading-order fixes) are harmless but unrelated to this feature.

Questions for the author

• Intended behavior when a client hits an event type it doesn't recognize (older client / newer server) — fail the whole read, or skip?
• Is List the intended long-term shape, or will a follow-up expose the underlying stream for large histories?